Implement support for both F_DISCARD and F_WRITE_ZEROES #464
Conversation
|
Fixes #360 |
|
This one needs an update of the imago crate. |
We need the latest version of the image crate to support F_DISCARD and F_WRITE_ZEROES. Signed-off-by: Sergio Lopez <[email protected]>
Extend VirtioBlkConfig enough to cover up to write_zeroes_may_unmap, as we're going to need those fields for implementing support both F_DISCARD and F_WRITE_ZEROES. Signed-off-by: Sergio Lopez <[email protected]>
Imago's discard_to_* operations require a mutable reference to Storage. Let's wrap disk_image in a Mutex so we can obtain such reference when needed. Since, in practice, we won't have multiple threads accessing the lock, the overhead should be very small (one or two operations on atomics). Signed-off-by: Sergio Lopez <[email protected]>
The F_DISCARD feature enables the guest to request us to free up sectors on the backing storage. Since imago does most of the heavy lifting, we just need to announce the feature and call to imago's "discard_to_any" then needed. Signed-off-by: Sergio Lopez <[email protected]>
The F_WRITE_ZEROES feature enables the guest to instruct the host to write zeroes to the disk image without actually having to copy any data around. Since imago does the heavy lifting for us, we just need to announce the feature and call to the right method when needed. Signed-off-by: Sergio Lopez <[email protected]>
slp
force-pushed
the
support-discard-zeroes
branch
from
95ff720 to
9149ac4
Compare
slp
requested review from
MatiasVara,
jakecorrenti,
mtjhrc and
tylerfanelli
as code owners
There was a problem hiding this comment.
You mention:
Since, in practice, we won't have multiple threads accessing the
lock, the overhead should be very small (one or two operations on
atomics).
Curious as to why we can be sure that multiple threads won't access the lock? Is there a dedicated thread for this?
I think that is accurate, there is a single dedicated worker thread for the device and the main event loop thread in libkrun that will end up activating the device. But still, I am a not really happy about adding the Mutex, because I think it should be avoidable if you are clear about the actual ownership of the disk object at any given time. Having had a quick look, to do this the object should be moved into the worker thread in I'm not really worried about the performance here, but the code quality if we were to keep adding mutexes to places where they shouldn't be necessary. |
3 participants
Imago supports both operations, so let's just enable them, as they may have a significant impact on both performance and disk usage on the host.